Skip to content

Move ISAAC generators to their own crate - #551

Merged
dhardy merged 1 commit into
rust-random:masterfrom
dhardy:isaac
Jul 16, 2018
Merged

dhardy merged 1 commit into
rust-random:masterfrom
dhardy:isaac

Conversation

@dhardy

@dhardy dhardy commented Jul 12, 2018

Copy link
Copy Markdown
Member

Part of #431

cargo doc --no-deps --all --all-features seems to fail in my tests unless I remove the --all-features flag. Not sure what's wrong here.

I left the PRNG comparisons in the prng module doc for now. We may decide to move that under doc/ later.

I also left the benchmarks as-is; this allows them to be compared across crates. I'm not sure whether to keep rand_isaac a dev-dependency just for this; perhaps eventually we should move the benchmarks.

For now the lib depends on rand_isaac anyway, just for the deprecation warnings (existing ones, and new ones for the names in the prng module).

@vks

vks commented Jul 12, 2018

Copy link
Copy Markdown
Contributor

I think the ISAAC generators should be removed from the table as well. Other than that, it looks good to me.

@dhardy

dhardy commented Jul 12, 2018

Copy link
Copy Markdown
Member Author

I'd like to have more comprehensive overviews of available PRNGs. These ISAAC generators are not quite CSPRNGs but that was where they fit best.

Okay, I admit, part of this is just not wanting to throw away all the work @pitdicker did on these implementations. 😄

@dhardy dhardy added the D-review Do: needs review label Jul 12, 2018
Comment thread rand_isaac/README.md Outdated

## Crate Features

`rand_isaac` supports `no_std` and `alloc`-only configurations, as well as full

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This crate always works with no_std, and only has the serde1 feature?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I forgot to update this bit. 👍

@pitdicker pitdicker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I admit, part of this is just not wanting to throw away all the work @pitdicker did on these implementations. smile

Git history preserves everything 😄. Still, having a good implementation of ISAAC in the ecosystem seems like good to have.

@dhardy

dhardy commented Jul 16, 2018

Copy link
Copy Markdown
Member Author

Git history preserves everything

Until you purge it! But discoverability and ease-of-use also matter I think.

@dhardy
dhardy merged commit 1a98577 into rust-random:master Jul 16, 2018
@dhardy
dhardy deleted the isaac branch July 16, 2018 10:11
@dhardy dhardy mentioned this pull request Sep 22, 2018
20 of 28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

D-review Do: needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants